Skip to content

[runtime] Reduce registered peer storage allocations - #12657

Merged
simonrozsival merged 1 commit into
mainfrom
simonrozsival-benchmark-registered-peers
Sep 3, 2026
Merged

[runtime] Reduce registered peer storage allocations#12657
simonrozsival merged 1 commit into
mainfrom
simonrozsival-benchmark-registered-peers

Conversation

@simonrozsival

Copy link
Copy Markdown
Member

Summary

  • store the first ReferenceTrackingHandle inline in each registered-peer dictionary entry
  • allocate a List<ReferenceTrackingHandle> only for actual JNI identity-hash collisions
  • use a dedicated Lock for registry synchronization
  • add BenchmarkDotNet coverage for registration, lookup, and collision remove/add churn

Benchmarks

BenchmarkDotNet 0.15.8, .NET 11 Arm64 RyuJIT, Apple M5 Max.

Add

Method PeerCount Mean Error StdDev Ratio Gen0 Gen1 Allocated Alloc Ratio
AlwaysList 1000 10.367 us 0.8682 us 0.0476 us 1.00 12.2986 3.0670 100.6 KB 1.00
FirstRest 1000 7.182 us 1.5371 us 0.0843 us 0.69 9.4299 1.8845 77.16 KB 0.77
ObjectOrList 1000 6.693 us 2.2895 us 0.1255 us 0.65 7.5150 1.2512 61.54 KB 0.61
InlineFirstRest 1000 3.689 us 0.4009 us 0.0220 us 0.36 5.7793 - 47.52 KB 0.47

Peek

Method PeersPerHash Mean Error StdDev Ratio Allocated
AlwaysList 1 2.750 ns 0.4352 ns 0.0239 ns 1.00 -
FirstRest 1 2.532 ns 0.1212 ns 0.0066 ns 0.92 -
ObjectOrList 1 3.414 ns 0.6184 ns 0.0339 ns 1.24 -
InlineFirstRest 1 1.947 ns 0.0188 ns 0.0010 ns 0.71 -
AlwaysList 4 4.359 ns 0.1926 ns 0.0106 ns 1.00 -
FirstRest 4 4.587 ns 0.0491 ns 0.0027 ns 1.05 -
ObjectOrList 4 5.401 ns 0.1493 ns 0.0082 ns 1.24 -
InlineFirstRest 4 4.228 ns 0.1335 ns 0.0073 ns 0.97 -

Collision remove/add churn

Method Mean Error StdDev Ratio Allocated
AlwaysList 7.993 ns 0.3645 ns 0.0200 ns 1.00 -
FirstRest 10.505 ns 0.5538 ns 0.0304 ns 1.31 -
ObjectOrList 10.362 ns 1.4922 ns 0.0818 ns 1.30 -
InlineFirstRest 17.718 ns 1.0713 ns 0.0587 ns 2.22 -

The churn case intentionally uses four peers for every identity hash. Real collision buckets are expected to be extremely rare and shallow; the singleton path avoids both boxing and list allocation.

Run with:

dotnet run --project tests/Xamarin.Android.Tools.Benchmarks/Xamarin.Android.Tools.Benchmarks.csproj -c Release -- --filter '*RegisteredPeers*'

Store the first registered peer inline and allocate a collision list only when identity hashes collide. Add BenchmarkDotNet coverage for registration, lookup, and collision churn across the candidate layouts.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 3, 2026 07:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The runtime changes appear internally consistent and safe, and the only feedback is minor benchmark-helper API contract tightening.

Review tier: Lite
Findings: 1 Low severity

New issues introduced by this change (1)
Severity Finding
Low severity tests/​Xamarin.Android.Tools.Benchmarks/​RegisteredPeersBenchmarks.cs — 💡 suggestion: This Peek implementation returns null on misses even though the method return…
What changed in this PR

Reduces allocation overhead in the runtime’s registered-peer registry by inlining the first ReferenceTrackingHandle per identity-hash bucket and only allocating a list on actual collisions, while also adding BenchmarkDotNet benchmarks to measure registration/lookup/churn behavior.

Changes:

  • Switch JavaMarshalRegisteredPeers storage from Dictionary<int, List<ReferenceTrackingHandle>> to Dictionary<int, RegisteredPeerBucket> and synchronize via a dedicated Lock.
  • Factor peer replacement/keep logic into ReconcilePeer() and ensure struct-bucket mutations are written back into the dictionary.
  • Add BenchmarkDotNet coverage for add/peek/remove-add churn scenarios across multiple storage strategies.
File Description
tests/​Xamarin.Android.Tools.Benchmarks/​RegisteredPeersBenchmarks.cs Adds new microbenchmarks and reference implementations comparing bucket storage strategies.
src/​Mono.Android/​Microsoft.Android.Runtime/​JavaMarshalRegisteredPeers.cs Implements inline-first bucket storage + dedicated lock for registered peers in the runtime.
Suppressed comments (3)

tests/Xamarin.Android.Tools.Benchmarks/RegisteredPeersBenchmarks.cs:311

  • 💡 suggestion: This Peek method can return null even though it returns Peer. If a miss is unexpected in this benchmark, throwing makes failures explicit without changing the hot path.
	public Peer Peek (Peer expected)
	{
		if (!peers.TryGetValue (expected.Hash, out FirstRestBucket values))
			return null;

tests/Xamarin.Android.Tools.Benchmarks/RegisteredPeersBenchmarks.cs:414

  • 💡 suggestion: Peek returns null on misses (including the single-handle fast path) even though the return type is Peer. Throwing on misses keeps the method contract non-null and surfaces setup errors clearly.
	public Peer Peek (Peer expected)
	{
		if (!peers.TryGetValue (expected.Hash, out object values))
			return null;

tests/Xamarin.Android.Tools.Benchmarks/RegisteredPeersBenchmarks.cs:476

  • 💡 suggestion: This Peek implementation returns null on misses while returning Peer. If misses are not expected in the benchmark harness, throwing makes failures explicit and avoids a misleading non-null signature.
	public Peer Peek (Peer expected)
	{
		if (!peers.TryGetValue (expected.Hash, out InlineFirstRestBucket values))
			return null;

Comment thread tests/Xamarin.Android.Tools.Benchmarks/RegisteredPeersBenchmarks.cs
@simonrozsival

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12657

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM

Findings: 0 errors · 0 warnings · 1 suggestion

The runtime bucket refactor preserves peer reconciliation, handle disposal, locking, and the required value-type dictionary write-backs. The dedicated lock also avoids coupling synchronization to the registry implementation. All 45 CI checks are green. I left one non-blocking inline suggestion to make the collision lookup benchmark exercise first/middle entries instead of only the reverse-scan fast case.

Generated by Android PR Reviewer for #12657 · gpt56 · 107.6 AIC · ⌖ 8.86 AIC · ⊞ 25.7K
Comment /review to run again

Comment thread tests/Xamarin.Android.Tools.Benchmarks/RegisteredPeersBenchmarks.cs
@simonrozsival simonrozsival added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Sep 3, 2026
@simonrozsival
simonrozsival enabled auto-merge (squash) September 3, 2026 13:26
@simonrozsival
simonrozsival merged commit 4b1fe7b into main Sep 3, 2026
45 checks passed
@simonrozsival
simonrozsival deleted the simonrozsival-benchmark-registered-peers branch September 3, 2026 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants